Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

v4.0.x: Prefer external hwloc and libevent #5472

Merged
merged 10 commits into from
Aug 15, 2018

Conversation

ggouaillardet
Copy link
Contributor

@ggouaillardet ggouaillardet commented Jul 24, 2018

(cherry picked from #5395 and #5466)

ggouaillardet and others added 4 commits July 24, 2018 09:52
Signed-off-by: Gilles Gouaillardet <gilles@rist.or.jp>
Signed-off-by: Jeff Squyres <jsquyres@cisco.com>

(cherry picked from commit open-mpi/ompi@ce2c9ff)
Switch from #-style to dnl-style.

Signed-off-by: Jeff Squyres <jsquyres@cisco.com>

(cherry picked from commit open-mpi/ompi@83e4a45)
Signed-off-by: Jeff Squyres <jsquyres@cisco.com>

(cherry picked from commit open-mpi/ompi@a70ecf5)
Signed-off-by: Ralph Castain <rhc@open-mpi.org>

(cherry picked from commit open-mpi/ompi@5cab823)
@hppritcha
Copy link
Member

bot:mellanox:retest

@hppritcha
Copy link
Member

bot:retest

@hppritcha hppritcha added the NEWS label Jul 30, 2018
@jsquyres jsquyres changed the title Prefer external hwloc and libevent v4.0.x: Prefer external hwloc and libevent Aug 7, 2018
Copy link
Member

@jsquyres jsquyres left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These commits nominally provide some of the requested functionality. But:

  1. Lots of warnings for external hwloc -- see Compiling with external hwloc results in many warnings #5524.
  2. There are cases with the external hwloc component that the external hwloc is not used but no explanation is provided as to why (but in other cases, a good explanation is provided).
  3. libevent and hwloc are quite different in their handling of internal vs. external. This may end up being confusing to users, and is going to lead to maintenance problems down the road, IMHO. I don't know how these two stack up against pmix's internal vs. external handling.
  4. README and/or NEWS needs to be updated to document this behavior.
  5. When the external components are selected, the configury of the internal components is confusing. From the output, it looks like the internal configury succeeds, but then it says it can't compile the component. Here's both hwloc and libevent internal configury outputs in this case:
checking whether hwloc configure succeeded... yes
checking for infiniband/verbs.h... (cached) yes
checking if MCA component hwloc:hwloc201 can compile... no
...
configure: /bin/sh './configure' succeeded for opal/mca/event/libevent2022/libevent
checking if MCA component event:libevent2022 can compile... no

At a minimum, I think #5524 needs to be solved before this PR is merged.

We know that hwloc:external will be configured first (because of its
priority).  Take advantage of that here in hwloc201 by having it
refuse to configure / politely fail if hwloc:external succeeded.

Also print out some additional lines in configure output indicating
what is going on (i.e., hwloc:external succeeded, so this component
will be skipped, or hwloc:external failed, so this component will be
used).

Signed-off-by: Jeff Squyres <jsquyres@cisco.com>
(cherry picked from commit 4e5f432)
We know that event:external will be configured first (because of its
priority).  Take advantage of that here in libevent2022 by having it
refuse to configure / politely fail if event:external succeeded.

Also print out some additional lines in configure output indicating
what is going on (i.e., event:external succeeded, so this component
will be skipped, or event:external failed, so this component will be
used).

Signed-off-by: Jeff Squyres <jsquyres@cisco.com>
(cherry picked from commit b063cb6)
@jsquyres
Copy link
Member

jsquyres commented Aug 9, 2018

Items 1 (warnings with external hwloc) and 5 (internal hwloc/libevent not configured -- and notice is given to explain why -- if external hwloc/libevent are selected) have been addressed with two new commits on this PR.

I think items 2 (more ?AC_MSG_NOTICEs? when external is not selected) and 4 (README+NEWS) should be addressed before this PR can be merged.

Given current resource constraints, item 3 (a wholesale revamp / unification of how internal-vs-external components are selected -- e.g., perhaps some common m4 between event/hwloc/pmix?) is outside the scope of this PR.


# These flags need to get passed to the wrapper compilers
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jsquyres this illustrates my latest comment in #5031

Change # -> dnl.  No code or logic changes.

Signed-off-by: Jeff Squyres <jsquyres@cisco.com>
(cherry picked from commit 17aa64e)
Put argument to AM_CONDITIONAL inside [].  No code or logic changes.

Signed-off-by: Jeff Squyres <jsquyres@cisco.com>
(cherry picked from commit 80df3f0)
In order to make "make distclean" (and friends) work, we need to
*always* invoke the embedded configure script -- even if we know that
we're not going to use this component.

But in cases where we know we're not going to use this component, we
also need to avoid the side effects of the code path that is used when
we *do* want to use this component.  So split the two possibilities
into two different macros:

1. MCA_opal_event_libevent2022_FAKE_CONFIG: which does almost nothing
   except invoke the underlying "configure" script.
2. MCA_opal_event_libevent2022_REAL_CONFIG: which does all the real
   work (including invoking the underlying "configure" script).

Signed-off-by: Jeff Squyres <jsquyres@cisco.com>
(cherry picked from commit 69aa46e)
The Autoconf AC_CONFIG_* macros can only be instantiated exacly once
for any given file, *and* they must be in a code execution path at run
time for the target file to be generated at the end of configure.

For example, if you want to generate file ABC at the end of configure,
you must invoke the AC_CONFIG_FILES(ABC) macro in a code path that
will get executed when configure is run.

That's pretty straightforward.

What's not straightforward is two corner cases:

1. You cannot invoke the AC_CONFIG_FILES(ABC) macro for the same file
   more than once.  If you do, autoreconf will fail (even before you
   can run configure).
2. If AC_CONFIG_FILES(ABC) is not in a code path that is executed by
   configure, the file ABC is not registered properly, and ABC will
   not be generated at the end of configure.

This applies to hwloc because hwloc's HWLOC_SETUP_CORE macro calls
both AC_CONFIG_FILES and AC_CONFIG_HEADER to setup its Makefiles
(etc.) so that targets like "make distclean" and "make distcheck" will
work properly.  Hence, we *have* to invoke HWLOC_SETUP_CORE.

However, the MCA_opal_hwloc_hwloc201_CONFIG macro has a few side
effects.  It would be nice to do able to do something like this:

```
    if hwloc:extern is going to be used:
        Invoke minimal HWLOC_SETUP_CORE (with no side effects)
    else
        Invoke full HWLOC_SETUP_CORE (with side effects)
    fi
```

But we can't, because autoreconf will detect that AC_CONFIG_FILES has
been invoked on the same files more than once (regardless of whether
those code paths will be executed at run time or not).  Kaboom.

Similarly, we can't do this:

```
    if hwloc:extern is not going to be used:
        Invoke full HWLOC_SETUP_CORE (with side effects)
    fi
```

Because then hwloc's AC_CONFIG_FILES won't be registered properly when
hwloc:external *is* used (i.e., when the HWLOC_SETUP_CORE macro is not
in a code path that is executed at run time), and targets like "make
distclean" will fail because hwloc's Makefiles won't have been setup.
Kaboom.

But remember that the hwloc framework is a bit special: there will
only ever be 2 comoponents: external and internal.  External is
guaranteed to be configured first because of its priority.  So the
internal component (i.e., this component) immediately knows if it is
going to be used or not based on whether the external component
configuration succeeded or failed.

Specifically: regardless of whether the internal component (i.e., this
component) is going to be used, we have to invoke HWLOC_SETUP_CORE.
But we can manage the side effects: allow the side effects when
this/internal component is going to be used, and avoid the side
effects when this/internal component is not going to be used.

This is a little less clean than I would have liked, but because of
Autoconf's oddity about its AC_CONFIG_* macros, this is the only
solution I could come up with.

Signed-off-by: Jeff Squyres <jsquyres@cisco.com>
(cherry picked from commit 01e4570)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants